Hi @alextech first of all. Awesome work. Thank you!

Secondly, I have created a 'external/alextech' branch for you to merge to. This should at least allow the PR to merge once you are done (I've removed the sonarqube requirement for 'external/*' target branches). I still have to approve the workflows to be run manually each change though. So just ping me here once you want another run.

If there is something else I can help with here then too let me know :-)

@alextech just wanted to inform you. I am overhauling some major parts of the internals. Which means everything will work with cucumber::messages::... to follow the official cucumber.io implementations.

This means reporters will receive a cucumber::messages::envelope argument for every type of message received. These cucumber::messages::... have native to-json support. and to my knowledge the cucumber messages use ndjson as well.

This will hopefully make your PR much much simpler!

@daantimmer Thank you for the update! will check out the changes. How difficult would it be to rebase external/alextech branch so that your changes do not look like they are new?

I am already using their native to-json support though it has indeed been a challenge to get to the cucumber::messages that are initially created during the AST parsing stage, so this should help.

@daantimmer Are you also planning on changing anything at the ReportHandlerV2::StepStart call ?
I was at the point where I realized that the ndjson objects which come on and after stepDefinition key come not from the compiler already prepared by the upstream parser, like the ones prior to it, but should be generated by the step matcher to have the location of the test definitions sources in the report.

I think while I wait for your overhaul I can work on ndjson runner comparator, the one that validates generated files against official compatibility versions.

@daantimmer Are you also planning on changing anything at the ReportHandlerV2::StepStart call

@alextech your assumption is correct. Reporters will be receiving 'cucumber::messages::envelope' messages. These will contain all the standardized information that you'll need to build any reporter.

Including pickle's/gherkin_documents/test_case's suite/case started/finished.

I've not finalized this design yet, but there will be something that is going to look like:

namespace cucumber_cpp::library::util
{
    struct Broadcaster;

    struct Listener
    {
        explicit Listener(Broadcaster& broadcaster, const std::function<void(const cucumber::messages::envelope& envelope)>& onEvent);
        ~Listener();

        void Invoke(const cucumber::messages::envelope& envelope) const;

    private:
        Broadcaster& broadcaster;
        std::function<void(const cucumber::messages::envelope& envelope)> onEvent;
    };

    struct Broadcaster
    {
        void AddListener(Listener* listener);
        void RemoveListener(Listener* listener);

        void BroadcastEvent(const cucumber::messages::envelope& envelope);

    private:
        std::vector<Listener*> listeners;
    };
}

So you get a reference to a Broadcaster and you can create a Listener and provide that listener a callback that will be called for every envelope received.

You can then do something like (pseudo, I haven't written a new reporter based on this yet):

struct ConsolePrinter
{
    ConsolePrinter(Broadcaster& broadcaster)
    : eventListener{broadcaster, [this](const cucumber::messages::envelope& envelope){ OnEvent(envelope); }}
    {}

    void OnEvent(const cucumber::messages::envelope& envelope)
    {
        if (envelope.test_case_started)
            handleTestCaseStarted(envelope.test_case_started);
        // etc
    }

    Listener eventListener;
};

An example of how one could write a 'PrettyPrinter' can be seen here: https://github.dev/cucumber/pretty-formatter/ I am following the same kind of behaviour and information provided.

I also have an implementation equal to that 'query' implementation available.

@daantimmer Using an event system will, indeed, make the architecture much simpler. Will probably have to restart PR given the fundamental changes, but in this case that is not a bad thing.

Out of curiosity, what are you using as a reference for what events need to be broadcasted when? This diagram: https://github.com/cucumber/messages/blob/main/jsonschema/relations.md ?

And would the Broadcaster need to be a global singleton or be passed around in deep DI chains? Wondering about this, because events can come from rather different namespaces of the system. Gherkin parser generates events, then step matcher, then each of the steps being executed.

Then, there are "overview" objects which are not strictly events. In the envelope they are gherkin_document, pickle, source, and meta. Meta is an interesting one because it would come from yet another utility (that I was going to add) that collects environment data about git branch, build details if ran from a pipeline, and version of the protocol used. Latter part I already did with a cmake trick that injects messages library version number into both a cpp source and into FetchContent cmake function.

Or maybe the Broadcaster event system should only apply to *_started / * _finished messages, while others can be captured similarly to how it is done now, but with easier access to the generated AST tree messages?

Exciting to see what you will come up with!

@alextech to give you an idea of the output that will be generated, this is from a valid test run:

https://gist.github.com/daantimmer/88fd8a8a0dddb73f005956b7c4740bc1

Note: not all the fields are currently filled and I don't have the meta in there yet.

The above output is generated by the following 'debug listener':

struct BroadcastListener
        {
            explicit BroadcastListener(util::Broadcaster& broadcaster)
                : listener(broadcaster, [this](const cucumber::messages::envelope& envelope)
                      {
                          OnEvent(envelope);
                      })
            {}

            void OnEvent(const cucumber::messages::envelope& envelope)
            {
                if (envelope.attachment)
                    std::cout << "on_attachment: " << envelope.attachment.value().to_json() << "\n";
                if (envelope.gherkin_document)
                    std::cout << "on_gherkin_document: " << envelope.gherkin_document.value().to_json() << "\n";
                if (envelope.hook)
                    std::cout << "on_hook: " << envelope.hook.value().to_json() << "\n";
                if (envelope.meta)
                    std::cout << "on_meta: " << envelope.meta.value().to_json() << "\n";
                if (envelope.parameter_type)
                    std::cout << "on_parameter_type: " << envelope.parameter_type.value().to_json() << "\n";
                if (envelope.parse_error)
                    std::cout << "on_parse_error: " << envelope.parse_error.value().to_json() << "\n";
                if (envelope.pickle)
                    std::cout << "on_pickle: " << envelope.pickle.value().to_json() << "\n";
                if (envelope.suggestion)
                    std::cout << "on_suggestion: " << envelope.suggestion.value().to_json() << "\n";
                if (envelope.source)
                    std::cout << "on_source: " << envelope.source.value().to_json() << "\n";
                if (envelope.step_definition)
                    std::cout << "on_step_definition: " << envelope.step_definition.value().to_json() << "\n";
                if (envelope.test_case)
                    std::cout << "on_test_case: " << envelope.test_case.value().to_json() << "\n";
                if (envelope.test_case_finished)
                    std::cout << "on_test_case_finished: " << envelope.test_case_finished.value().to_json() << "\n";
                if (envelope.test_case_started)
                    std::cout << "on_test_case_started: " << envelope.test_case_started.value().to_json() << "\n";
                if (envelope.test_run_finished)
                    std::cout << "on_test_run_finished: " << envelope.test_run_finished.value().to_json() << "\n";
                if (envelope.test_run_started)
                    std::cout << "on_test_run_started: " << envelope.test_run_started.value().to_json() << "\n";
                if (envelope.test_step_finished)
                    std::cout << "on_test_step_finished: " << envelope.test_step_finished.value().to_json() << "\n";
                if (envelope.test_step_started)
                    std::cout << "on_test_step_started: " << envelope.test_step_started.value().to_json() << "\n";
                if (envelope.test_run_hook_started)
                    std::cout << "on_test_run_hook_started: " << envelope.test_run_hook_started.value().to_json() << "\n";
                if (envelope.test_run_hook_finished)
                    std::cout << "on_test_run_hook_finished: " << envelope.test_run_hook_finished.value().to_json() << "\n";
                if (envelope.undefined_parameter_type)
                    std::cout << "on_undefined_parameter_type: " << envelope.undefined_parameter_type.value().to_json() << "\n";
            }

        private:
            util::Listener listener;
        };

Hey @alextech you can follow along with the improvements/implementation of cucumber messages here: #273

@daantimmer That was an exciting merge conflict ) I stepped through the execution of minimal.test and it seems to cover almost everything I was planning to do here. Even validation of ndjson with compatibility seems to be coming. Maybe the only thing that could remain useful is the report destructor fix and even then may not be needed.

I like the Broadcaster::BroadcastEvent call at the end of each operation + OnEnvelope to pick up the events. It does fix every problem I had trying capturing the message/envelope. Though JavaScript in me subconsciously keeps looking for Onfor every envelope combo, centralization is just as convenient. And having a listener for the purpose of comparing expected vs actual is clever.

Further ideas I may have should be done against the new version so I think I will close this PR.